refactor: standardize srt-slurm v2.2.1 and migrate recipes - #3086
Conversation
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs 感谢你的贡献！请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md，然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人，你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板，包括保留英文语句 如需进行 PR 验证，请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
|
Claude finished @cquil11's task in 8m 51s —— View job Review of PR #3086
SummaryNo blocking issues found. Master config edits are covered by a One 🟡 warning posted inline: the eval-override test does not validate the Not run locally due to sandbox restrictions: the pytest suites and the srt-slurm clone the new |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding, I double-checked the ruled-out concern that the schema-2 role-env injector (runners/synthetic_injectors/_roles.py) only matches roles: anchored at column 0 — relevant because the six base:-wrapped multi-variant recipes this PR carries forward (e.g. b200-fp4/8k1k.yaml, glm5-mtp.yaml x2, glm5.2-agentx.yaml x2) nest roles: under base:/override_* at indent 2, not column 0. I confirmed in configs/nvidia-master.yaml that none of the matrix scenarios referencing those six files via their override_*/zip_override_* selectors currently set SYNTHETIC_ACCEPTANCE=true (MTP variants there route through separately-resolved standalone recipe files instead), so the injector gap is real but latent, not exercised by today's matrix.
Extended reasoning...
This is the narrow findings-present exception: a confirmed CI-trigger-path finding is already posted inline, so my role here is limited to noting what else was checked and ruled out. I independently re-verified one of the two previously ruled-out candidates (the _roles.py column-0 roles: anchor) against the actual recipe files this PR adds/keeps and against configs/nvidia-master.yaml, since the fragment I was given implied it might affect real files. It does affect the YAML structure of six specific recipes, but none of the matrix entries that select MTP variants from those particular files currently enable SYNTHETIC_ACCEPTANCE, so the gap does not fire under the present configuration — consistent with the prior rule-out, now corroborated with a concrete code-path trace rather than just restated.
4991903 to
3cb1c1f
Compare
f84071a to
7a10a8c
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
939f532 to
96f33f7
Compare
cquil11
changed the title
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f47df86. Configure here.
42a322e to
312dcf8
Compare
Migrate and organize srt-slurm recipes; restore required EPLB assets and original dedicated-service placement. Integrate the reviewed recipe changes into the runtime standardization branch. Upstream power compatibility remains a blocker for the combined PR to main.
cquil11
changed the title
Preserve SGLang Model Gateway behavior with the renamed sglang-router frontend. Keep the documented power compatibility limitations for maintainer follow-up.
cquil11
changed the title
1 participant
Standardize NVIDIA Slurm launchers on srt-slurm v2.2.1, migrate the owned recipes, and organize them under consistent model-first paths. Recipe PR #3103 has been integrated into this branch.
984180e5b8755aef85e9995048b5a16cb5336bce) throughutils/srt-slurm; preserve the existing AIPerf submodule.6bc3f306bdafa1edfb5dded2fcda8f1ccede1bde, with no separate submodule.<model-prefix>/<engine>/<gpu>-<precision>/<workload>/<recipe>.yaml. Update active/deprecated master references, launcher path rules, workflow filters, and English/Chinese documentation together.Validation: 263 focused launcher, injector, power and matrix tests passed during implementation. Recipe organization preserves all 618 generated multinode rows after path substitution and all 45 expanded variants across six override bundles. The restored service blocks match fresh migration of their originals. Three independent reviewers assessed #3103 before its integration into this development branch; their approval of that integration does not establish production readiness.
Known power limitations retained for PowerX maintainer follow-up: the pinned upstream runtime rejects 18 NVIDIA recipes with dedicated-infrastructure power telemetry (697 of 715 NVIDIA files pass validation). Multinode AgentX power also requires measurement-window metadata that the pin does not provide. TileRT passed its fork validator. Hardware validation has not run. Preexisting internal recipe-name inconsistencies and worker-only scheduling estimates remain outside this migration's scope.
No performance changelog entry or new srt-slurm test workflow is included, as requested.
Release upgrade validation: 263 focused tests pass with v2.2.1; all 618 generated multinode matrix entries are unchanged. Eleven SGLang recipes now explicitly select
sglang-routerto preserve their original Model Gateway behavior after the upstream frontend rename. The same 18 dedicated-infrastructure telemetry validation failures remain. The user explicitly requested merging with the known power limitations and will notify the PowerX maintainer. No hardware sweep was run.